Conversation
|
@brandur actually what's your thought on whether this should just be part of |
So if the user specifies their own set of indexes to reindex and then uses Pro, Pro would append one list to the other? I guess this should be fine. The only the downside I can imagine is if you people end up coming to rely on it too much, they may request additional customization like number of concurrent reindexes, etc., and before you know it you could a more expansive set of options needed (we already have two on there just for the reindexer and hopefully it wouldn't expand anymore than that). I suppose that's not the end of the world though. It'd be sort of nice to have a way of marking a property as "contemplative, but not totally stable" in Go to give us a little leeway to change this sort of thing, but I don't think it's possible. |
bgentry
force-pushed
the
bg/reindexer-pro-extension
branch
from
62d45d4 to
b5767f9
Compare
bgentry
changed the title
Some Pro features add high churn tables that would benefit from the
reindexer. As of now, however, there's no way to extend it to run on
anything but the default index list.
Add `Config.ReindexerIndexNames` so callers can provide the exact list
to reindex, and export `ReindexerIndexNamesDefault()` so integrations
can start from the built-in targets without reaching into internal
maintenance code.
Thread the configured names into `maintenance.Reindexer` and filter
them through `IndexesExist` before starting work. That lets
mixed-version or partially migrated installs skip absent indexes
instead of trying to rebuild objects that are not there. Preserve
the `nil` versus non-`nil` contract in `WithDefaults` by copying the
slice without collapsing an explicit empty override back to `nil`, so
`[]string{}` still means "reindex nothing".
When `IndexesExist` fails during reindex discovery, advance the next
scheduled run before resetting the timer. The old code reset against
the already-fired `nextRunAt`, which made `time.Until(nextRunAt)`
zero or negative and caused immediate retries in a tight error loop.
Scheduling from the prior run time preserves the configured cadence
after transient discovery failures. The added tests cover the exported
default list, exact override propagation, explicit empty overrides,
missing-index filtering, and the discovery-error retry path.
bgentry
force-pushed
the
bg/reindexer-pro-extension
branch
from
b5767f9 to
c67ca19
Compare
|
@brandur I rewrote this to add a The main downside in this new implementation is the extra API to expose the default list and the potential for drift if you don't reference it and instead try to copy-paste the list's values. Speaking of which, those values are tucked into an internal package and not immediately available for click-through. I doubt many users will customize this to begin with, so it's probably not much of an issue. Thoughts? |
2 participants
The reindexer currently operates on a fixed list of indexes, which makes it hard to extend in deployments that have additional high-churn indexes worth maintaining.
Add
Config.ReindexerIndexNamesso callers can provide the exact reindex target list, and exportReindexerIndexNamesDefault()so integrations can compose from River's built-in defaults without reaching into internal maintenance code. Preserve the public contract thatnilmeans "use the defaults" while a non-nilslice is the exact override, including[]string{}meaning "reindex nothing".The reindexer now filters configured targets through
IndexesExist, warns when configured targets are missing, and advances the schedule after discovery errors instead of retrying immediately in a tight error loop. Tests cover the exported default list, override propagation, explicit empty overrides, missing-index filtering, and the discovery-error retry path.